refactor: switch to Astro #25
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR (Simeonoff/v2) removes a significant amount of legacy SassDoc theme and navigation code while introducing an Astro configuration for static site generation.
- Removal of SassDoc theme integration and related navigation JavaScript files
- Removal of the gulpfile and build tasks associated with the legacy setup
- Addition of a new Astro configuration file and an updated LICENSE with a new year
Reviewed Changes
Copilot reviewed 131 out of 141 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sassdoc/index.js | Removed SassDoc theme integration using themeleon |
| sassdoc/assets/js/versioning/tag-versions.req.js | Removed legacy versioning code |
| sassdoc/assets/js/navigation/*.js | Removed legacy navigation related files |
| gulpfile.js | Removed legacy Gulp build configuration |
| astro.config.ts | Added new Astro configuration for static site generation |
| LICENSE | Updated copyright year |
Files not reviewed (10)
- .browserlistrc: Language not supported
- .editorconfig: Language not supported
- .jshintrc: Language not supported
- .prettierrc: Language not supported
- package.json: Language not supported
- sassdoc/config.json: Language not supported
- sassdoc/scss/base/_base.scss: Language not supported
- sassdoc/scss/base/_normalize.scss: Language not supported
- sassdoc/scss/base/_typography.scss: Language not supported
- sassdoc/scss/main.scss: Language not supported
Comments suppressed due to low confidence (3)
sassdoc/index.js:1
- Removal of the SassDoc theme integration file may disrupt documentation rendering if any consumers still reference it. Please ensure that all dependencies or configuration references related to themeleon are updated or removed.
-/**
gulpfile.js:1
- Removal of gulpfile.js might break the build automation and development workflow if no alternative build process is provided. Confirm that the new build configuration (e.g., from Astro or other tooling) fully replaces the functionality offered by the gulpfile.
-'use strict';
sassdoc/assets/js/navigation/igviewer.common.js:1
- The removal of igviewer.common.js may lead to broken navigation-related functionality if any part of the project still expects its behavior. Ensure that all references to this module are updated or that its functionality has been superseded by new code.
-(function (window, $, Modernizr) {
|
@dobromirts Do you think you will have time to at least glance over the code here? I can defer this to someone else if you don't have the capacity. I don't expect you to do a thorough review since it's a massive departure from the original implementation, just some sanity checks and a second pair of eyes. |
|
I will be able to do some sanity checks, but I’ll need a few days before I have the capacity to look into it, most likely on Friday. If it’s time sensitive, feel free to pass it to someone else, otherwise I’m happy to take a look once I get the chance. |
.github/workflows/npm-publish.yml
Outdated
| run: if [[ ${VERSION} == *"alpha"* || ${VERSION} == *"beta"* || ${VERSION} == *"rc"* ]]; then echo "NPM_TAG=next"; else echo "NPM_TAG=latest"; fi >> $GITHUB_ENV | ||
| - run: echo ${NPM_TAG} | ||
| run: | ||
| if [[ ${VERSION} == *"alpha"* || ${VERSION} == *"beta"* || ${VERSION} == *"rc"* ]]; then |
There was a problem hiding this comment.
is this change required? Isnt it this change going to cause issue due to use of echo ${NPM_TAG} immediately after setting it via $GITHUB_ENV, because echo "NPM_TAG=..." >> $GITHUB_ENV sets the variable for future steps, not in the current one.
There was a problem hiding this comment.
I reverted the changes.
|
There are several differences between the infragistics prod navigation and the current one (font sizes and styles)- |
|
In order to verify all sass data and all links to the github project reference, this project should be linked in the library right? And only for visual comparison dev and server commands from the package.json are used based on the mocked test/library?
|
- Add missing charset meta tag for better document encoding - Replace inline Google Tag Manager iframe with proper noscript fallback - Add CSS styles to properly hide the GTM iframe with reusable mixins - Improve accessibility and code structure for GTM implementation
- Change working directory for npm version and publish commands from ./ to dist - Ensure package version and publishing actions run in the correct directory
- Add translations for header action buttons in English and Japanese - Update ItemHeader component with better search metadata attributes - Remove content-visibility CSS property from Item component - Fix import paths for i18n utilities
- Add SCSS build and watch scripts - Create post-build script to copy package.json to dist - Pre-compile SCSS modules to CSS - Move igniteui-theming to devDependencies and upgrade to v18.0.1
Correct. |
We do a lot less processing to the navigation now. The navigation and footer should look closer to the source we're working with. |
src/i18n/ja.json
Outdated
There was a problem hiding this comment.
@randriova Can you please verify that the Japanese strings in this file are correct? You can clone this repo and run npm install && npm run dev to get a bit more context about where those strings are used in the project.
There was a problem hiding this comment.
@randriova If you need a bit of help setting up this project, let me know. I can give you a quick call and help.
Closes #24